MDEV-40180 Cache constant args in JSON_EQUALS and JSON_OVERLAPS - #5455
MDEV-40180 Cache constant args in JSON_EQUALS and JSON_OVERLAPS#5455VasuBhakt wants to merge 1 commit into
Conversation
* Optimize JSON_EQUALS by maintaining DYNAMIC_STRING class members to cache constant arguments natively within val_bool(), preventing redundant re-parsing and eliminating memory allocation overhead by reusing the buffers across rows. * Fix JSON_OVERLAPS constant caching, which was improperly cached in fix_length_and_dec (causing --ps-protocol failures), by moving it to val_bool(). * Ensure cleanup() correctly resets caching state for both functions for prepared statements. * Note: This commit intentionally skips the nested objects short-circuit logic, which will be implemented incrementally in a future update. Signed-off-by: VasuBhakt <cpswastik31@gmail.com>
1a7bfa7 to
d137c8b
Compare
grooverdan
left a comment
grooverdan
left a comment
There was a problem hiding this comment.
didn't get time to look at overlaps properly.
with all faults of json_equals - needs test case of an actual table with null/nonnul and constant comparison like
| } | ||
| else | ||
| { | ||
| a= args[0]->val_json(&a_tmp); |
| dynstr_free(&a_res); | ||
| null_value= 1; | ||
| return 1; | ||
| if(a_null) |
|
|
||
| if (!cached_a.str) | ||
| { | ||
| if (init_dynamic_string(&cached_a, NULL, 0, 0)) |
There was a problem hiding this comment.
As this is going to contain a normalized form of a, use a->length() as the 3rd arg for the initial size.
The failure of this is a memory allocation failure. While it hasn't been done well in other examples in this file yet, the response is:
my_error(ER_OUTOFMEMORY, MYF(0), a->length());
goto return_null
As a pushed error it shouldn't return here
| } | ||
| else | ||
| { | ||
| cached_a.length= 0; |
There was a problem hiding this comment.
just a comment in code here about resetting string for next value.
| DYNAMIC_STRING b_res; | ||
| if (init_dynamic_string(&b_res, NULL, 0, 0)) | ||
| /* Process First Argument */ | ||
| if (a_const && a_parsed) |
There was a problem hiding this comment.
a_parsed is always the same as cached_a.str != nullptr so I think a_parsed can be eliminated.
| goto return_null; | ||
| if (a_const) | ||
| { | ||
| a_null= false; |
There was a problem hiding this comment.
I think can set a_null unconditionally. Its only looked at under a_const ==true anyway0.
|
|
||
| result= strcmp(a_res.str, b_res.str) ? 0 : 1; | ||
| /* Process Second Argument */ | ||
| if (b_const && b_parsed) |
There was a problem hiding this comment.
same comments on first arg parsing apply here too.
| String *v= args[1]->val_json(&tmp_val); | ||
| if (v) | ||
| { | ||
| cached_val.copy(v->ptr(), v->length(), v->charset()); |
There was a problem hiding this comment.
still copying here.
if (b_const)
{
if (cached_val.is_allocated())
val= &cached_val; /* or something*/
else
{
val= args[1]->val_json(&cached_val);
}
}
else
{
something using local val= tmp_str (as local)
}
(incomplete)
2 participants
Fixes MDEV-40180
Problem
JSON_EQUALSevaluates and parses constant arguments per-row.JSON_OVERLAPScaches constant arguments infix_length_and_dec, which fails to recalculate during--ps-protocolexecutions with changing bound parameters.Solution
Migrated constant argument caching logic to
::val_bool()for both functions:Item_func_json_equals: Added deep-copy caching of the normalizedDYNAMIC_STRINGupon first evaluation of a constant argument. Subsequent evaluations bypassjson_normalize_engineand reuse the cached string.Item_func_json_overlaps: Removed flaweda2_constantcaching fromfix_length_and_decand implemented per-execution caching of the evaluated string insideval_bool().cleanup()in both classes to reset boolean caching flags, ensuring correct cache invalidation between executions.(Note: The nested object short-circuit evaluation will be addressed in a follow-up incremental PR).
Tests
Executed the
json,json_equals, andjson_normalizeMTR suites (both standard and--ps-protocol) to verify behavior preservation and resolution of the prepared statement cache bug.Version
The change is directed at versions
10.11,11.4,11.8, and12.3. The PR is based on branch10.11, being the lowest affected branch.